Skip to content

template: Add break_words option to fill function - #10079

Open
genericusername2709 wants to merge 1 commit into
jj-vcs:mainfrom
genericusername2709:main
Open

template: Add break_words option to fill function#10079
genericusername2709 wants to merge 1 commit into
jj-vcs:mainfrom
genericusername2709:main

Conversation

@genericusername2709

@genericusername2709 genericusername2709 commented Aug 29, 2026

Copy link
Copy Markdown

Add an option of specifying if we want to break words when using fill function in jj log template
Sample template to test:
jj log --template "fill(89, self.description()) ++ \"\n\""

Added functionality to allow breaking words that are longer than the length passed in fill function in jj log templates

WHY: To allow proper rendering of commit messages on thin terminal windows using custom templates.

Checklist

If applicable:

  • I have updated the documentation (README.md, docs/, demos/)
  • I have added/updated tests to cover my changes
  • I fully understand the code that I am submitting (what it does,
    how it works, how it's organized), including any code drafted by an LLM.
  • For any prose generated by an LLM, I have proof-read and copy-edited with
    an eye towards deleting anything that is irrelevant, clarifying anything
    that is confusing, and adding details that are relevant. This includes,
    for example, commit descriptions, PR descriptions, and code comments.

@genericusername2709
genericusername2709 requested a review from a team as a code owner August 29, 2026 11:15

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The following commits do not follow our format for subject lines:

  • 4bc0d67: Add an option of specifying if we want to break words when using fill function in jj log template

Commits should have a subject line following the format <topic>: <description>. Please review the commit guidelines for more information.

@github-actions
github-actions Bot dismissed their stale review August 29, 2026 11:24

All commits are now correctly formatted. Thank you for your contribution!

@genericusername2709 genericusername2709 changed the title Add an option of specifying if we want to break words when using fill function in templates cli: Add an option of specifying if we want to break words when using fill function in templates Aug 29, 2026
@genericusername2709

Copy link
Copy Markdown
Author

Testing word-breaking:
image

@josephlou5 josephlou5 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you maybe need a changelog entry?

Also, move explanation of the change (not just "how to test") into the commit description per https://docs.jj-vcs.dev/latest/contributing/#commit-guidelines.

Comment thread docs/templates.md Outdated
Comment thread docs/templates.md Outdated
@josephlou5

Copy link
Copy Markdown
Contributor

Also your commit subject is a bit long :) Suggestion:

template: Add break_words option to fill function

Note that these template functions can be used in many commands, not just jj log.

@sbroglie

Copy link
Copy Markdown

Generally, please do not use booleans as a parameters. This is the Boolean Blindness anti-pattern.
Use a constant instead, e.g. (feel free to bikeshed the case and name)

fill(80, self.description(), BREAK_LONG_LINES)
fill(80, self.description(), break.LONG_LINES) -- if there is something like a namespace

This makes the template more descriptive and the effect/intent is obvious to a reader. It also avoids confusion if there is more than one boolean argument: fill(..., true, false) was the first boolean breaking long lines or right-justify?

As a bonus it is more flexible for future changes, e.g. add more "modes" like justifying, breaking with a dash "-", ...

@PhilipMetzger

Copy link
Copy Markdown
Contributor

Use a constant instead, e.g. (feel free to bikeshed the case and name)

There neither are constants or enums in the template language, so this will be hard.

@josephlou5

Copy link
Copy Markdown
Contributor

Generally, please do not use booleans as a parameters. This is the Boolean Blindness anti-pattern.

IIUC, the code looks for a named parameter here, so break_words=true should work. Could ask the author to add a test proving this, but I don't think this is a huge issue here. Generally I'd agree about booleans being not ideal, but the alternative in this templating DSL is probably to have another function fill_break_words, and I dislike that more (if we add more options later, we'll have to make a bunch of new functions to accommodate all possible options).

@josephlou5

Copy link
Copy Markdown
Contributor

Use a constant instead, e.g. (feel free to bikeshed the case and name)

There neither are constants or enums in the template language, so this will be hard.

For completeness, this could be done, but I wouldn't recommend it:

[template-aliases]
FILL_BREAK_WORDS = 'true'

'format_commit(commit)' = '''
fill(50, commit.description(), FILL_BREAK_WORDS)
'''

Any user could come by and override their own template-aliases.FILL_BREAK_WORDS to equal 'false' (or even whatever else they want), so it's not a true constant.

You could also:

[template-config]
# This table doesn't exist in the schema, but you can define whatever you want
# in your config; jj does not error in this case (yet).
fill_break_words = true

[template-aliases]
'format_commit(commit)' = '''
fill(50, commit.description(), config("template-config.fill_break_words").as_boolean())
'''

But this moves function arguments into a config, which sounds weird. FWIW I do use this pattern in some of my template aliases for "default function arguments", but I don't think it makes sense for the upstream project to ship with this pattern.

@yuja

yuja commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Generally, please do not use booleans as a parameters.

For public Rust functions, I agree.

Maybe we can add a separate step for splitting long words? We can also add a dedicated template function for this step (instead of extending fill()), but I have no strong preference.

@genericusername2709 genericusername2709 changed the title cli: Add an option of specifying if we want to break words when using fill function in templates template: Add break_words option to fill function Aug 30, 2026
@genericusername2709

Copy link
Copy Markdown
Author

Generally, please do not use booleans as a parameters.

For public Rust functions, I agree.

Maybe we can add a separate step for splitting long words? We can also add a dedicated template function for this step (instead of extending fill()), but I have no strong preference.

Adding new functions for each variation in functionality can spiral into a bloated function list quickly.

Added a test for a template that uses the named param break_words=true

@genericusername2709
genericusername2709 force-pushed the main branch 2 times, most recently from 584ec9b to 0f62867 Compare August 30, 2026 01:52
This change adds a optional bool param to the globally available fill function
in templating DSL to allow users to specify if they want to split words if the
size of any word in the input is greater than the `width` passed to the
function.


Current supported template:
`jj log --template "fill($(($(tput cols) - 8)), self.description()) ++ \"\n\""`

After this change: (The original keeps being supported)
`jj log --template "fill($(($(tput cols) - 8)), self.description(), break_words=true) ++ \"\n\""
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants